-
Notifications
You must be signed in to change notification settings - Fork 104
Conversation
This commit adds basic support for helm template. The functionality allows one to either render a chart's templates to a specific directory, or capture the rendered templates as a string of concatenated yaml documents.
Codecov Report
@@ Coverage Diff @@
## main #368 +/- ##
==========================================
- Coverage 36.80% 35.81% -1.00%
==========================================
Files 3 3
Lines 758 779 +21
Branches 148 154 +6
==========================================
Hits 279 279
- Misses 430 451 +21
Partials 49 49
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of small docs nits, the Python parts of the module lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add something to the module docs about idempotency, we talked in slack and it sounds like the CLI is not idempotent so we should just note that so that users expecting Ansible's usual behaviour aren't surprised.
Could we add check mode support to this module? I'm not really familiar enough with how folks use Helm and would expect that to work, but in theory we could still build the command with --dry-run
(or even just store the rendered yaml to stdout?).
output_dir: "{{ temp_dir }}" | ||
values_files: | ||
- "{{ role_path }}/files/values.yaml" | ||
register: result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to have some assertions for this task. is not failed, result.rc value, and result.command should be straightforward, depending on the reproducability and complexity of result.stdout there should be something we can assert about the shape or contents that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, examining stdout here will be difficult. This test suite gets run multiple times with different charts so the output will be different for each one. The same thing goes for examining result.command
to some extent. I've added a check for as much of the command as we can reliably predict.
plugins/modules/helm_template.py
Outdated
|
||
module.exit_json( | ||
failed=False, | ||
changed=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I missed this the first time around, shouldn't changed always be True?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Thanks @gravesm
This repository does not accept pull requests, see the README for details. |
SUMMARY
This commit adds basic support for helm template. The functionality
allows one to either render a chart's templates to a specific directory,
or capture the rendered templates as a string of concatenated yaml
documents.
Closes #367
ISSUE TYPE
COMPONENT NAME
helm_template
ADDITIONAL INFORMATION
Sorry, for the extensive diff on
tests_chart.yml
but I really wanted to fix our test teardown. Aside from indentation changes, the only lines in that file that are new are 3-11 and 325-349.